Generic cells in doppel#663
Merged
Merged
Conversation
8b9e7dd to
89d1b37
Compare
c7878a4 to
4bad0ff
Compare
89d1b37 to
b3a0238
Compare
4bad0ff to
1469947
Compare
b3a0238 to
8b10ba0
Compare
5c809af to
856c262
Compare
8b10ba0 to
379e0b9
Compare
856c262 to
7f67b46
Compare
379e0b9 to
511a371
Compare
7f67b46 to
bb9d369
Compare
511a371 to
356f019
Compare
labbott
approved these changes
May 20, 2026
356f019 to
a45b581
Compare
bb9d369 to
327fc04
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Load derive proc-macro and the humility_doppel “cell” mirror types to support typed/generic cells (e.g., StaticCell<Ringbuf>), then updates call sites to use the new typed cells and simplify value extraction.
Changes:
- Extend
#[derive(Load)]code generation to include generic parameters for derived struct impls. - Make
doppelcell wrapper types (ClaimOnceCell,StaticCell,UnsafeCell,MaybeUninit) generic over the loaded payload type and remove the bespokeLoadimpl forMaybeUninit<T>. - Update
host,ringbuf, andspdto use typed cell reads (viareflect::read_variable) and avoid redundantfrom_valuecalls.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
load_derive/src/lib.rs |
Adds helper functions to emit generics in generated impl Load blocks for derived structs. |
humility-doppel/src/lib.rs |
Makes cell wrapper types generic and switches MaybeUninit<T> to #[derive(Load)] instead of a manual impl. |
humility-spd/src/lib.rs |
Uses typed ClaimOnceCell<reflect::Struct> loading to simplify packrat buffer handling and removes manual loading steps. |
cmd/ringbuf/src/lib.rs |
Uses StaticCell<Ringbuf> to avoid re-parsing the ringbuf after unwrapping the cell. |
cmd/host/src/lib.rs |
Uses ClaimOnceCell<HostStateBuf> and returns the typed inner value directly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
327fc04 to
e0f22cf
Compare
759562f to
a88a132
Compare
e0f22cf to
2740bf7
Compare
a88a132 to
cecefd8
Compare
e363734 to
ff617b0
Compare
ff617b0 to
2856341
Compare
2856341 to
d10f467
Compare
Contributor
Author
|
Tested on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(staged on #662)
This PR makes cell types in
doppelgeneric, then uses that new functionality where relevant.It requires editing the proc macro to support generic parameters, but then can remove the explicit
impl Load for MaybeUninit<T>implementation.